Repository navigation
feat(sql-editor): browse nested BI connections - #111178
Conversation
|
[Medium risk] Adds nested table browsing to the BI SQL editor. The PR appears safe to merge; no new actionable issue or outstanding previous finding was identified. Reviews (2) · Last reviewed commit: "fix(data-warehouse): refine BI connectio..." |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Important Review skippedWe couldn't safely recover the incremental review. No full review was started, and the last reviewed checkpoint was preserved. Retry later, or explicitly request a full review by commenting You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughBI mode now displays linked tables and views, loads their fields on demand, and supports nested connections and path-aware fields. Users can search connections and add connected fields to Rows. The changes also add connection-tree tests and a Storybook scenario, document the behavior, and add 🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
Full details: Description checkExplanation The description covers the problem, user-visible changes, testing, release status, docs, and agent context. It omits required before-and-after Mermaid flowcharts for the connection and loading-state changes, and the agent context does not include a session link or report the duplicate-PR and patch-coverage checks. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (2)
products/data_warehouse/frontend/bi/biConnectionTree.ts-75-75 (1)
75-75: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse the canonical hydration key for BI connections.
The sidebar uses
table.idfor PostHog tables andtable.namefor other tables. The BI tree currently usestable.namefor both status lookup and hydration. If these values differ, the connection can remain inloading.Suggested fix
+ const getHydrationTableKey = (table?: DatabaseSchemaTable): string | undefined => + table ? (table.type === 'posthog' ? table.id : table.name) : undefined const visit = (tableName: string, fields: DatabaseSchemaField[], path: string[]): BIConnection[] => fields .flatMap((originalField): BIConnection[] => { @@ const childPath = [...path, originalField.name] const id = JSON.stringify(childPath) const table = getTable(field.table) - const status = table ? tableFieldsStatus[table.name] : undefined + const hydrationTableKey = getHydrationTableKey(table) + const status = hydrationTableKey ? tableFieldsStatus[hydrationTableKey] : undefined const state = getConnectionState(field, table, status, databaseFieldsComplete) const connection: BIConnection = { @@ - tableName: table?.name, + tableName: hydrationTableKey,products/data_warehouse/frontend/bi/BIConnectionGroup.tsx-44-57 (1)
44-57: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winGive the missing state a next step.
When
connection.state === 'missing', the UI only reports that fields are unavailable. Tell the user how to resolve it, such as checking that the linked table still exists. Theerrorbranch already offersRetry.Suggested copy
- This connection's fields are unavailable. + Fields for this connection aren't available. Check that the linked table still exists.
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
- Review profile: QUIET
- Plan: Enterprise
- Run ID:
ac9d7b57-d7be-42fd-9f60-85b6ad271f90
📒 Files selected for processing (13)
docs/published/handbook/engineering/data-warehouse.mdfrontend/src/scenes/data-warehouse/editor/OutputPane.tsxfrontend/src/scenes/data-warehouse/editor/SQLEditorScene.stories.tsxfrontend/src/scenes/data-warehouse/editor/bi/biEditorTypes.tsfrontend/src/scenes/data-warehouse/editor/bi/components/BIDataPane.tsxfrontend/src/scenes/data-warehouse/editor/sidebar/queryDatabaseLogic.tsxfrontend/src/scenes/data-warehouse/editor/sqlEditorLogic.test.tsproducts/data_warehouse/frontend/bi/BIConnectionGroup.tsxproducts/data_warehouse/frontend/bi/BIConnections.tsxproducts/data_warehouse/frontend/bi/BIDataPaneSection.tsxproducts/data_warehouse/frontend/bi/biConnectionTree.test.tsproducts/data_warehouse/frontend/bi/biConnectionTree.tsproducts/data_warehouse/frontend/bi/biConnectionsLogic.ts
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 8 remain after this review.
🤖 CI report✅ Trunk lane — non-backend laneThis PR is assigned to the non-backend lane. It does not run backend Python tests and may merge in parallel with PRs in other lanes.
|
| Function | Location | Complexity | Limit |
|---|---|---|---|
parseBIEditorState |
frontend/src/scenes/data-warehouse/editor/bi/biEditorTypes.ts:431 |
38 | 10 |
<anonymous> |
frontend/src/scenes/data-warehouse/editor/sidebar/queryDatabaseLogic.tsx:3434 |
30 | 10 |
InternalDataTableVisualization |
frontend/src/scenes/data-warehouse/editor/OutputPane.tsx:1025 |
26 | 10 |
resolveFieldTraverserTarget |
frontend/src/scenes/data-warehouse/editor/sidebar/queryDatabaseLogic.tsx:586 |
24 | 10 |
buildBIQuery |
frontend/src/scenes/data-warehouse/editor/bi/biEditorTypes.ts:994 |
23 | 10 |
OutputPane |
frontend/src/scenes/data-warehouse/editor/OutputPane.tsx:625 |
22 | 10 |
getBIFilterSummary |
frontend/src/scenes/data-warehouse/editor/bi/biEditorTypes.ts:679 |
22 | 10 |
filterExpression |
frontend/src/scenes/data-warehouse/editor/bi/biEditorTypes.ts:724 |
22 | 10 |
createFieldNode |
frontend/src/scenes/data-warehouse/editor/sidebar/queryDatabaseLogic.tsx:1058 |
21 | 10 |
Content |
frontend/src/scenes/data-warehouse/editor/OutputPane.tsx:1263 |
19 | 10 |
getBIChartFit |
frontend/src/scenes/data-warehouse/editor/bi/biEditorTypes.ts:327 |
18 | 10 |
<anonymous> |
products/data_warehouse/frontend/bi/biConnectionTree.ts:76 |
18 | 10 |
BIDataPane |
frontend/src/scenes/data-warehouse/editor/bi/components/BIDataPane.tsx:15 |
16 | 10 |
RenderInfo |
frontend/src/scenes/data-management/database/DatabaseTable.tsx:176 |
15 | 10 |
parseBIFieldValue |
frontend/src/scenes/data-warehouse/editor/bi/biEditorTypes.ts:399 |
15 | 10 |
<anonymous> |
frontend/src/scenes/data-warehouse/editor/bi/biEditorTypes.ts:490 |
15 | 10 |
createTopLevelFolderNode |
frontend/src/scenes/data-warehouse/editor/sidebar/queryDatabaseLogic.tsx:1605 |
15 | 10 |
getHydrationTableNamesForNode |
frontend/src/scenes/data-warehouse/editor/sidebar/queryDatabaseLogic.tsx:318 |
14 | 10 |
<anonymous> |
frontend/src/scenes/data-warehouse/editor/sidebar/queryDatabaseLogic.tsx:3168 |
14 | 10 |
createPropertyDefinitionChildren |
frontend/src/scenes/data-warehouse/editor/sidebar/queryDatabaseLogic.tsx:419 |
13 | 10 |
createSourceFolderNode |
frontend/src/scenes/data-warehouse/editor/sidebar/queryDatabaseLogic.tsx:1536 |
13 | 10 |
renderCell |
frontend/src/scenes/data-warehouse/editor/OutputPane.tsx:780 |
12 | 10 |
<anonymous> |
frontend/src/scenes/data-warehouse/editor/OutputPane.tsx:248 |
11 | 10 |
ResultsActions |
frontend/src/scenes/data-warehouse/editor/OutputPane.tsx:465 |
11 | 10 |
createViewNode |
frontend/src/scenes/data-warehouse/editor/sidebar/queryDatabaseLogic.tsx:1386 |
11 | 10 |
<anonymous> |
frontend/src/scenes/data-warehouse/editor/sidebar/queryDatabaseLogic.tsx:3846 |
11 | 10 |
✅ Duplication (Python) — clean
New Python code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.
✅ Duplication (TypeScript) — clean
New TypeScript code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.
⚠️ Bundle size — 🔺 +5.7 KiB (+0.0%)
Uncompressed size of every built .js bundle, compared against the base branch.
Total: 69.52 MiB · 🔺 +5.7 KiB (+0.0%)
| File | Size | Δ vs base |
|---|---|---|
exporter/src/exporter/scenes/ExporterNotebookScene.js |
3.93 MiB | 🔺 +5.7 KiB (+0.1%) |
Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report
✅ Eager graph — within budget
How much code each root ships on the eager path — downloaded and parsed before the surface is interactive. Measured from the esbuild output chunks (post-tree-shake, static imports only); lazy import() / React.lazy chunks are not counted.
| Root | Eager (shipped) | Δ vs base | Budget |
|---|---|---|---|
entry (logged-out pages, app bootstrap)src/index.tsx |
1.63 MiB · 23 files | no change | █████████░ 88.6% of 1.84 MiB |
logged-out boot: index + App + bootApp (preloaded by every page, including /login)src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts |
3.71 MiB · 663 files | no change | █████████░ 92.2% of 4.03 MiB |
authenticated shell (every logged-in page)src/scenes/AuthenticatedShell.tsx |
7.61 MiB · 2,415 files | no change | █████████░ 91.3% of 8.34 MiB |
dashboard scenesrc/scenes/dashboard/Dashboard.tsx |
9.70 MiB · 3,446 files | no change | ███████░░░ 72.0% of 13.48 MiB |
today home pathsrc/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.tsx |
7.62 MiB · 2,423 files | no change | █████████░ 88.8% of 8.58 MiB |
events scenesrc/scenes/activity/explore/EventsScene.tsx |
9.32 MiB · 3,298 files | no change | ███████░░░ 73.7% of 12.64 MiB |
replay detail scenesrc/scenes/session-recordings/detail/SessionRecordingDetail.tsx |
12.13 MiB · 4,142 files | no change | ████████░░ 77.2% of 15.72 MiB |
🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 node_modules/monaco-editor/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/layout/navigation-3000/navigationLogic.tsx stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/scenes/dashboard/dashboardLogic.tsx stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/lemon-ui/LemonMarkdown/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/components/RichContentEditor/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/components/CodeSnippet/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/taxonomy/core-filter-definitions-by-group.json stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 node_modules/monaco-editor/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/scenes/session-recordings/player/sessionRecordingPlayerLogic.ts stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/scenes/project-homepage/ai-first/AiFirstHomepage.tsx stays out of src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.tsx
🟢 src/scenes/project-homepage/today/TodayReportPage.tsx stays out of src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.tsx
Largest files eagerly shipped from src/index.tsx
| Size | File |
|---|---|
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 24.6 KiB | ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js |
| 6.3 KiB | ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js |
| 4.5 KiB | ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js |
| 3.9 KiB | ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js |
| 1.4 KiB | ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js |
| 1.3 KiB | src/index.tsx |
| 1.3 KiB | src/RootErrorBoundary.tsx |
| 912 B | ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js |
| 839 B | src/scenes/ChunkLoadErrorBoundary.tsx |
Largest files eagerly shipped from src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
| Size | File |
|---|---|
| 306.2 KiB | ../node_modules/.pnpm/posthog-js@1.435.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 220.3 KiB | ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 99.7 KiB | src/lib/api.ts |
| 90.2 KiB | src/products.tsx |
| 69.4 KiB | src/lib/lemon-ui/icons/icons.tsx |
| 40.4 KiB | src/lib/utils/eventUsageLogic.ts |
| 38.7 KiB | ../node_modules/.pnpm/@dnd-kit+core@6.0.8_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@dnd-kit/core/dist/core.esm.js |
| 33.9 KiB | ../node_modules/.pnpm/kea@4.0.0-pre.6_patch_hash=139b8d1f1304f9d9da452a9a1244c94ea679dbcb85687d8999563146879fb6f5_react@18.3.1/node_modules/kea/lib/index.cjs.js |
| 29.0 KiB | ../node_modules/.pnpm/zod@4.3.6/node_modules/zod/v4/core/schemas.js |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
| Size | File |
|---|---|
| 306.2 KiB | ../node_modules/.pnpm/posthog-js@1.435.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 280.3 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 220.3 KiB | ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 153.7 KiB | ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 112.1 KiB | ../packages/quill/packages/quill/dist/index.js |
| 99.7 KiB | src/lib/api.ts |
| 93.3 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
| 90.6 KiB | ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js |
| 90.2 KiB | src/products.tsx |
Largest files eagerly shipped from src/scenes/dashboard/Dashboard.tsx
| Size | File |
|---|---|
| 306.2 KiB | ../node_modules/.pnpm/posthog-js@1.435.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 280.3 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 220.3 KiB | ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 181.8 KiB | src/queries/validators.js |
| 153.7 KiB | ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 112.1 KiB | ../packages/quill/packages/quill/dist/index.js |
| 99.7 KiB | src/lib/api.ts |
| 93.3 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
| 90.6 KiB | ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.tsx
| Size | File |
|---|---|
| 306.2 KiB | ../node_modules/.pnpm/posthog-js@1.435.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 280.3 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 220.3 KiB | ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 153.7 KiB | ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 112.1 KiB | ../packages/quill/packages/quill/dist/index.js |
| 99.7 KiB | src/lib/api.ts |
| 93.3 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
| 90.6 KiB | ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js |
| 90.2 KiB | src/products.tsx |
Largest files eagerly shipped from src/scenes/activity/explore/EventsScene.tsx
| Size | File |
|---|---|
| 306.2 KiB | ../node_modules/.pnpm/posthog-js@1.435.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 280.3 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 220.3 KiB | ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 181.8 KiB | src/queries/validators.js |
| 153.7 KiB | ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 112.1 KiB | ../packages/quill/packages/quill/dist/index.js |
| 99.7 KiB | src/lib/api.ts |
| 93.3 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
| 90.6 KiB | ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js |
Largest files eagerly shipped from src/scenes/session-recordings/detail/SessionRecordingDetail.tsx
| Size | File |
|---|---|
| 315.5 KiB | ../node_modules/.pnpm/posthog-js@1.435.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/rrweb.js |
| 306.2 KiB | ../node_modules/.pnpm/posthog-js@1.435.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 280.3 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 220.3 KiB | ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 181.8 KiB | src/queries/validators.js |
| 153.7 KiB | ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 112.1 KiB | ../packages/quill/packages/quill/dist/index.js |
| 99.7 KiB | src/lib/api.ts |
| 93.3 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479
✅ Toolbar bundle — eager 2.21 MiB within budget
What the toolbar ships to customer pages, measured from the esbuild output (minified, post-tree-shake). The eager set is the entry plus everything statically imported from it — fetched before any feature runs; deferred chunks load lazily. The eager guardrail is 5.72 MiB. Each output file must also stay below 10 MB, where CloudFront stops compressing it. The module boundary is enforced separately by check-toolbar-graph.
| Metric | Size | Δ vs base | Budget |
|---|---|---|---|
| Eager (shipped) entry + static imports |
2.21 MiB · 19 files | no change | ████░░░░░░ 38.6% of 5.72 MiB |
| Deferred (lazy) | 2.11 MiB · 44 files | no change | n/a — loads on demand |
Loader dist/toolbar.js |
1.2 KiB | no change | █░░░░░░░░░ 6.0% of 19.5 KiB |
Largest eagerly-shipped chunks
| Size | File |
|---|---|
| 843.7 KiB | dist/toolbar/toolbar-app-7WP5DJ5V.css |
| 657.1 KiB | dist/toolbar/chunk-chunk-ANDLGWTP.js |
| 259.4 KiB | dist/toolbar/chunk-chunk-6KW5GTW2.js |
| 138.2 KiB | dist/toolbar/chunk-chunk-TELGYDIR.js |
| 131.8 KiB | dist/toolbar/chunk-chunk-FDH2IBXT.js |
| 75.2 KiB | dist/toolbar/toolbar-app-CPK5ZUBF.js |
| 69.0 KiB | dist/toolbar/chunk-chunk-TSAL54PB.js |
| 35.6 KiB | dist/toolbar/chunk-chunk-K5C4YQSL.js |
| 21.0 KiB | dist/toolbar/chunk-chunk-ZAZMQ4LE.js |
| 6.8 KiB | dist/toolbar/chunk-chunk-DV7IWQNF.js |
Posted automatically by check-toolbar-size · sizes are toolbar output bytes (shipped, post-tree-shake) from the esbuild metafile
✅ Dist folder size — 🔺 +103.4 KiB (+0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 960.76 MiB · 🔺 +103.4 KiB (+0.0%)
ℹ️ MCP UI apps size — 33 app(s), 17633.6 KB JS
Built size of each MCP UI app (main.js + styles.css).
| App | JS | CSS |
|---|---|---|
| debug | 597.9 KB | 199.4 KB |
| action | 454.1 KB | 199.4 KB |
| action-list | 564.2 KB | 199.4 KB |
| cohort | 453.1 KB | 199.4 KB |
| cohort-list | 563.2 KB | 199.4 KB |
| email-template | 452.9 KB | 199.4 KB |
| error-details | 469.6 KB | 199.4 KB |
| error-issue | 454.5 KB | 199.4 KB |
| error-issue-list | 564.8 KB | 199.4 KB |
| experiment | 561.3 KB | 199.4 KB |
| experiment-list | 564.9 KB | 199.4 KB |
| experiment-results | 566.3 KB | 199.4 KB |
| feature-flag | 566.8 KB | 199.4 KB |
| feature-flag-list | 570.5 KB | 199.4 KB |
| feature-flag-testing | 457.3 KB | 199.4 KB |
| inline-scan | 453.6 KB | 199.4 KB |
| insight-actors | 562.3 KB | 199.4 KB |
| invite-email-preview | 452.3 KB | 199.4 KB |
| llm-costs | 559.3 KB | 199.4 KB |
| session-recording | 455.3 KB | 199.4 KB |
| survey | 454.7 KB | 199.4 KB |
| survey-global-stats | 561.9 KB | 199.4 KB |
| survey-list | 564.9 KB | 199.4 KB |
| survey-stats | 561.9 KB | 199.4 KB |
| trace-span | 453.5 KB | 199.4 KB |
| trace-span-list | 564.1 KB | 199.4 KB |
| vision-observation-list | 563.3 KB | 199.4 KB |
| workflow | 453.4 KB | 199.4 KB |
| workflow-list | 563.5 KB | 199.4 KB |
| loops-review | 457.8 KB | 199.4 KB |
| query-results | 774.3 KB | 199.4 KB |
| render-ui | 858.4 KB | 199.4 KB |
| visual-review-snapshots | 457.9 KB | 199.4 KB |
5496082 to
625f848
Compare
|
There was a problem hiding this comment.
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (2)
docs/published/handbook/engineering/data-warehouse.md-246-246 (1)
246-246: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDocument the destination for measures.
Double-click and Enter send connected measures to Values, not Rows:
getBIDropTargetredirects measures there. State that dimensions go to Rows and measures go to Values.products/data_warehouse/frontend/bi/BIDataPaneSection.tsx-77-77 (1)
77-77: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winHandle native click activation without double-adding fields.
BIConnectionGrouprenders connected fields through this button, but the add action runs only on double-click or Enter. Touch taps and Space activation triggerclickwithout adding the field. UseonClickfor native activation and ignore the second click in a double-click sequence.Suggested fix
- onDoubleClick={() => addField(field)} - onKeyDown={(event) => { - if (event.key === 'Enter') { + onClick={(event) => { + if (event.detail < 2) { addField(field) } }}
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
- Review profile: QUIET
- Plan: Enterprise
- Run ID:
691c2738-dd7f-4468-a684-0402efd66ea5
📒 Files selected for processing (4)
docs/published/handbook/engineering/data-warehouse.mdproducts/data_warehouse/frontend/bi/BIConnectionGroup.tsxproducts/data_warehouse/frontend/bi/BIConnections.tsxproducts/data_warehouse/frontend/bi/BIDataPaneSection.tsx
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 8 remain after this review.
625f848 to
2e2e8a8
Compare
6a834ef to
c5d580e
Compare
|
Risk: No findings This delta is a correctness fix in the BI query builder: bounds the last-7-days filter to the past (excluding future timestamps), deduplicates dimension aliases instead of silently shadowing them, and emits series-breakdown chart settings for two-dimension bar/line/area charts. It is frontend-only HogQL construction that runs under the querying user's own permissions, with no auth, tenancy, or injection surface. Sentinel reviewed |
🦔 Hogbox preview · ✅ ready▶ Open the preview
commit |
👀 Auto-assigned reviewersThese soft owners were skipped because they only have minor changes here. Nothing blocks merge, so self-assign if you'd like a look:
Soft owners come from each directory's |
|
😎 Stack merged successfully - details. |
5ea9b4d to
665fcb6
Compare
|
🕓 This approval covered an earlier revision. There are new visual changes to review in the newer comment below. ✅ Visual changes approved by @mariusandra — baseline updated in 2 changed, 2 new. Install the Visual Review Chrome extension to see visual review results at the top of your pull requests. |
0966005 to
1f5b00d
Compare
4 updated Run: 47d17024-29f2-42ee-be8a-ec4027873f19 Co-authored-by: mariusandra <53387+mariusandra@users.noreply.github.com>
1f5b00d to
74f46ef
Compare
HostHog preview —
|
|
👋 Visual changes detected for this PR. Review and approve in PostHog Visual Review If these changes are unexpected, they may be caused by a flaky test or a broken snapshot on master. Don't approve — rerun the job or wait for a fix. Install the Visual Review Chrome extension to see visual review results at the top of your pull requests. |
0c76f30 to
14bc6df
Compare
A new stamphog review started for this PR — the fresh verdict replaces this approval.
There was a problem hiding this comment.
Approved.
Frontend-only additive feature behind the SQL_EDITOR_BI_MODE flag, with tests and a Storybook story. It touches no risky territory, and the author has STRONG familiarity with the code. The reviewer-bot comments were minor and have been addressed in later commits.
- Author wrote 97% of the modified lines and has 142 merged PRs in these paths (familiarity STRONG).
- 👍 on the PR from greptile-apps[bot].
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 793L, 12F substantive, 1103L/15F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (1103L, 15F, cross-cutting, feat) |
| stamphog 2.3.1 | .stamphog/policy.yml @ 14bc6df · reviewed head 14bc6df |
Problem
BI users can browse their selected table’s fields, but must open the SQL sidebar to reach linked tables.
This extends #111172 with connected fields directly in the BI data pane.
Changes
The shared field-row component supports keyboard activation with Enter and Space while retaining double-click and drag interactions.
The implementation uses the existing database schema API. This PR changes no files under
posthog/hogqland adds no schema fields.Narrow layout after dragging connected fields
Retry remains visible during search
How did you test this code?
Virtual connection using the existing schema
Focused Jest coverage exercises connected fields, SQL sidebar traversal, and SQL editor hydration. TypeScript, formatting, and preflight check the frontend-only diff.
Browser checks confirm two nested connections render at 1280px and 1050px, and virtual fields render without inferred links.
Storybook uses synthetic schema responses; live database joins were not executed.
Test rationale: Connection tests cover paths, cycles, loading states, and virtual fields incorrectly inheriting parent-table types or links. The existing SQL editor suite covers hydration without changing the source. The connections story captures two nested levels.
👉 Stay up-to-date with PostHog coding conventions for a smoother review.
Release status
SQL_EDITOR_BI_MODEDocs update
Updated the existing data warehouse engineering guide with nested fields, expansion, search, and retry behavior.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Agent: Codex, GPT-6
Tools: Git, GitHub CLI, Flox, hogli, Jest, and Playwright. Skills: debugging-ci-failures, fixing-flaky-tests, stacking-prs, placing-product-frontend-code, writing-ui-components, writing-kea-logics, writing-user-facing-copy, writing-tests, running-ci-preflight, reviewing-with-coderabbit, and writing-pr-descriptions.
CodeRabbit CLI was skipped because it remains signed out after the earlier setup offer. All fixtures and screenshots use synthetic Storybook data. The backend schema extension was removed, and conflicts with master were resolved while preserving both SQL editor stories.